Skip to content

Lower Demo app deployment target#84

Merged
Caiopia merged 2 commits intomasterfrom
host-cell-equality
Apr 25, 2018
Merged

Lower Demo app deployment target#84
Caiopia merged 2 commits intomasterfrom
host-cell-equality

Conversation

@Caiopia
Copy link
Copy Markdown
Member

@Caiopia Caiopia commented Apr 24, 2018

What

This adds key and style to the equality definition of CellConfigType. While most style changes are accompanied by state changes, it's not always the case for indexed items that change positions. Adding style makes it more likely we update cells when they change their merged style. This also allows us to compare CellConfigTypes more accurately outside of FunctionalTableData.

Bonus ✨

  • Allow demo app to be used with iOS 9.0+
  • 🔥 1 warning

@Caiopia Caiopia requested a review from raulriera as a code owner April 24, 2018 14:55
@Caiopia Caiopia requested review from ElisaKazan and g-Off April 24, 2018 14:55
Copy link
Copy Markdown
Contributor

@ElisaKazan ElisaKazan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread FunctionalTableData/HostCell.swift Outdated
var isEqual = key == other.key
isEqual = isEqual && state == other.state
isEqual = isEqual && style == other.style
return isEqual
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to just && the conditions together now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe the issue that led you to here. The keys and styles are compared separately by TableSectionChangeSet so I would have thought that comparing state was enough here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main reason is that if we want to equate CellConfigTypes outside of the TableSectionChangeSet, it wouldn't take style into account. Though I'm currently revisiting our use case to see if we really do need to equate these.

@Caiopia Caiopia force-pushed the host-cell-equality branch from 0c8bc50 to e6fbc48 Compare April 25, 2018 21:19
@Caiopia
Copy link
Copy Markdown
Member Author

Caiopia commented Apr 25, 2018

Removing the HostCell equality, I don't need it anymore. Now it just removes a warning and the 11.3 demo app restriction.

@Caiopia Caiopia merged commit 8488d15 into master Apr 25, 2018
@Caiopia Caiopia deleted the host-cell-equality branch April 25, 2018 22:37
@Caiopia Caiopia changed the title Host cell equality Lower Demo app deployment target Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants